fix: don't treat ignored build scripts as failure during dep install#1237
Merged
Conversation
Contributor
📦 Bundle Stats —
|
| Metric | Value | vs main (73eb7a2) |
|---|---|---|
| Internal (raw) | 2.1 KB | - |
| Internal (gzip) | 799 B | - |
| Bundled (raw) | 11.13 MB | - |
| Bundled (gzip) | 2.10 MB | - |
| Import time | 860ms | -54ms, -5.9% |
bin:sanity
| Metric | Value | vs main (73eb7a2) |
|---|---|---|
| Internal (raw) | 1023 B | - |
| Internal (gzip) | 486 B | - |
| Bundled (raw) | 9.87 MB | - |
| Bundled (gzip) | 1.77 MB | - |
| Import time | 1.94s | -96ms, -4.7% |
🗺️ View treemap · Artifacts
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
📦 Bundle Stats — @sanity/cli-core
Compared against main (73eb7a2a)
| Metric | Value | vs main (73eb7a2) |
|---|---|---|
| Internal (raw) | 96.3 KB | - |
| Internal (gzip) | 22.7 KB | - |
| Bundled (raw) | 21.70 MB | - |
| Bundled (gzip) | 3.45 MB | - |
| Import time | 762ms | -5ms, -0.7% |
🗺️ View treemap · Artifacts
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
📦 Bundle Stats — create-sanity
Compared against main (73eb7a2a)
| Metric | Value | vs main (73eb7a2) |
|---|---|---|
| Internal (raw) | 908 B | - |
| Internal (gzip) | 483 B | - |
| Bundled (raw) | 931 B | - |
| Bundled (gzip) | 491 B | - |
| Import time | ❌ ChildProcess denied: node | - |
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
Contributor
Coverage Delta
Comparing 1 changed file against main @ Overall Coverage
|
There was a problem hiding this comment.
Pull request overview
Adjusts Sanity CLI’s dependency installation flow to avoid failing onboarding when pnpm exits non-zero due to ignored build scripts (eg ERR_PNPM_IGNORED_BUILDS), while keeping real install failures as errors.
Changes:
- Run package manager installs with
execa(..., {reject: false})and interpret results based on exit status + output. - For pnpm, treat
ERR_PNPM_IGNORED_BUILDSas success and optionally warn users to runpnpm approve-builds(suppressed for esbuild-only). - Add/extend tests to cover ignored-build-script scenarios across
installDeclaredPackagesandinstallNewPackages.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/@sanity/cli/src/util/packageManager/installPackages.ts | Adds pnpm ignored-build handling and switches installs to reject: false. |
| packages/@sanity/cli/src/util/packageManager/tests/installPackages.test.ts | Adds test coverage for pnpm ignored-build outputs and updates expectations for reject: false. |
| .changeset/pr-1237.md | Adds a patch changeset entry for @sanity/cli. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
rexxars
changed the title
rexxars
force-pushed
the
fix/pnpm-build-scripts
branch
from
7f2f092 to
3452a2f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
SDK-1654
When running
npm create sanity/sanity init, if selecting pnpm to use for installing dependencies you can get into this situation where pnpm installs dependencies, but exits with a non-zero exit code, and a message like:We need to allow this to happen - treating it as if this was a success (which it kind of is), instead of failing the onboarding process. Since esbuild works fine for the vast majority of cases without running the postinstall script, I decided to not show the "Run 'pnpm approve-builds'" text if that is the only dependency that needs it - but open to changing/removing that, just figured it would be less noise.
Note
pnpm create sanityhas a similar problem, but during thepnpxstage. This is harder to fix, but might be solved if/when #1011 landsWhat to review
Does the change seem sensible?
Testing
New tests added.
Note
Low Risk
Scoped to CLI package install error handling for pnpm; real install failures still exit with an error, with broader test coverage.
Overview
Fixes pnpm dependency installs during Sanity CLI onboarding (
sanity init/npm create sanity) when pnpm exits non-zero afterERR_PNPM_IGNORED_BUILDSeven though packages were installed.installPackagesnow runs package-manager commands withreject: false, detects ignored-build-script output (including wrapped/whitespace-separated lists), and treats that case as success. If only esbuild was skipped, no extra message is shown; for other packages (e.g. sharp, scoped names), it warns to runpnpm approve-builds. Real failures still fail the spinner but now log stdout and stderr so errors on stderr are visible. Behavior is unchanged for npm/yarn/bun.Tests cover the new pnpm paths plus updated
execaexpectations; changeset is a patch for@sanity/cli.Reviewed by Cursor Bugbot for commit 3452a2f. Bugbot is set up for automated code reviews on this repo. Configure here.